[dev-v5] Add Drag & Drop component #4211
Conversation
There was a problem hiding this comment.
The components must have @attributes in their razor files and use the v5 default class and style builders. With the way it is done now, unit tests are failing (because the standard parameters are not being utilized.
The components must be added in the ComponentInitializer dictionary in ComponentBaseTests.cs like this:
{ typeof(FluentDragContainer<>), Loader.MakeGenericType(typeof(string))},
{ typeof(FluentDropZone<>), Loader.MakeGenericType(typeof(string))},
Also, a lot more tests need to be added. We are strict on having 98% line coverage for v5. Current result:
So yes, all properties/parameters must have tests. And also yes, we didn;t have any additional plans for these components for V5.
|
@vnbaaij I have added your requested changes. Any idea why the ComponentBaseTest is failing? |
You forgot to use default builders in drag container cs file |
oh wow. I forgot to save the file. Should be done now. I will add more UNIT tests later on. Can you tell me how I can see the coverage for the specific component? Nevermind - the docs got me covered! 🤫 |
|
@vnbaaij I need your wisdom once again. This testing stuff is kinda new to me - so please show mercy! 🤣 How would you do it for the Draggable property? I guess I shouldn't declare two methods for true and false, right? And how can I even check for mouse events? The docs shows how you can do it with If you could give me like 1 or 2 examples then I will happily add the rest of the tests. |
| /// <summary> | ||
| /// Gets the zone where the drag started. | ||
| /// </summary> | ||
| public FluentDropZone<TItem> Source { get; } = null!; |
There was a problem hiding this comment.
Do not use null!, but use required instead if necessary.
|
|
||
| /// <summary /> | ||
| [CascadingParameter] | ||
| private FluentDragContainer<TItem> Container { get; set; } = default!; |
There was a problem hiding this comment.
Prefer to use required and not default!
| } | ||
|
|
||
| div[dragged-over] { | ||
| background-color: lightgray; |
There was a problem hiding this comment.
Do we need to use a FluentUI CSS variable (and not always lightgray) ?
There was a problem hiding this comment.
Good point. Which one would you recommend?
|
|
||
| <!-- Log before modification --> | ||
| <Message Importance="normal" Text="Found @(_ContentWithAbsolutePaths->Count()) Content items with absolute paths to normalize" /> | ||
| <Message Importance="normal" Text="Found @(_ContentWithAbsolutePaths->Count()) Content items with absolute paths to normalize" /> |
There was a problem hiding this comment.
Visual Studio doing Visual Studio things. I change it back.
| @inherits Bunit.TestContext | ||
|
|
||
| @code { | ||
| public FluentDragTests() |
There was a problem hiding this comment.
You will probably need to add additional unit tests to achieve at least 98% code coverage (100% if possible).
There was a problem hiding this comment.
Yea I know. Could you help me out by providing a testcase for the Draggable parameter? See: #4211 (comment)
I'd need to look into this on how to do it. Maybe you can draw inspiration from |
Pull Request
📖 Description
This PR adds the
FluentDragContainerandFluentDropZonecomponents from v4 branch to v5. They are basically a copy & paste with just small changes to get them working in v5.I also added the demo section and adjusted both test cases.
🎫 Issues
Implements #4208
👩💻 Reviewer Notes
@vnbaaij & @dvoituron not sure if you wanted to change something fundamental here in v5.
📑 Test Plan
✅ Checklist
General
Component-specific
⏭ Next Steps
I think in v5 we want to add test cases for each property right?